roborev: Combined Review

Verdict: Not ready to merge — there is one high-severity auth exposure risk plus multiple medium-severity correctness/availability issues.

High

1. Potential unauthenticated access to sensitive API endpoints
   • Location: internal/api/server.go (routes), validated via internal/api/handlers_test.go
   • Details: New sensitive routes POST /auth/token/{email} and POST /accounts appear reachable without API-key auth middleware; one review notes tests exercising access without an API key.
   • Risk: Unauthorized token overwrite/account creation.
   • Fix: Ensure both routes are mounted under the API-key-protected router group in setupRouter.

Medium

1. Request cancellation/timeouts bypassed in query-heavy handlers

   • Location: internal/api/handlers.go:~900, ~940, ~970, ~1010/~1020, ~1060/~1070, ~1100
   • Details: Handlers (handleAggregates, handleSubAggregates, handleFilteredMessages, handleTotalStats, handleFastSearch, handleDeepSearch) use context.Background() instead of r.Context().
   • Risk: Expensive work continues after client disconnect/timeout, increasing DoS exposure.
   • Fix: Pass r.Context() through all engine/DB calls and ensure downstream honors cancellation.

2. Config save is unsafe (non-atomic + permissions may remain too broad)

   • Location: internal/config/config.go:243, internal/config/config.go:257
   • Details: Config.Save() writes with os.O_TRUNC directly to config.toml; crash/ENOSPC can corrupt file. Also, existing files may retain permissive mode because open flags alone do not tighten permissions.
   • Risk: Config loss/corruption; potential secret exposure if mode is too open.
   • Fix: Write to temp file in same dir, fsync, rename atomically, then enforce 0600 (e.g., explicit chmod).

3. Remote deep-search query reconstruction is lossy

   • Location: internal/remote/engine.go:487, internal/remote/engine.go:611
   • Details: Rebuilding raw query strings drops semantics (quoted phrases unquoted; fields like AccountID/HideDeleted omitted).
   • Risk: Remote search results diverge from local behavior (broader or incorrectly filtered results).
   • Fix: Send structured filters to /search/deep (preferred) or fully preserve quoting/escaping and all filter fields.

4. search --json output schema differs between local and remote modes

   • Location: cmd/msgvault/cmd/search.go:158, cmd/msgvault/cmd/search.go:183
   • Details: Local returns array; remote returns object envelope { total, results }.
   • Risk: Script/API consumer breakage when switching to remote mode.
   • Fix: Normalize to one stable schema across modes (or gate new schema behind an explicit flag/version).

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

roborev: Combined Review

Verdict: Changes are not ready to merge due to 1 High and 4 Medium-severity issues.

High

1. Credential leakage risk on HTTP redirects
   • Locations: internal/remote/store.go:66, internal/remote/store.go:91, cmd/msgvault/cmd/export_token.go:149, cmd/msgvault/cmd/export_token.go:181, cmd/msgvault/cmd/export_token.go:223
   • Default http.Client redirect behavior can forward X-API-Key (and for 307/308, potentially replay sensitive request bodies) to unintended targets.
   • Recommended fix: Set CheckRedirect to fail closed (or only allow strict same-origin redirects with scheme/host validation).

Medium

1. Unbounded limit enables resource exhaustion

   • Locations: internal/api/handlers.go:414, internal/api/handlers.go:~520, internal/api/handlers.go:533, internal/api/handlers.go:~600, internal/api/handlers.go:642
   • limit is accepted without a hard upper bound in filter/aggregate paths, allowing very large requests (DB/memory/response-size DoS risk).
   • Recommended fix: Enforce a max limit (for example 500), reject oversized values with 400, and add tests.

2. Config save path can preserve insecure file permissions (and is non-atomic)

   • Locations: internal/config/config.go:243, internal/config/config.go:257
   • Save() uses direct O_TRUNC writes; existing permissive modes can persist and expose secrets (api_key), and truncation is not atomic.
   • Recommended fix: Write temp file with 0600, fsync, rename, then enforce secure chmod on final path.

3. Remote search query serialization changes query semantics

   • Locations: internal/remote/engine.go:469, internal/remote/engine.go:505, internal/remote/engine.go:620
   • Rebuilding parsed queries into raw strings without proper quoting/escaping can alter behavior (especially quoted phrases / values with spaces).
   • Recommended fix: Preserve original raw query string end-to-end or implement correct escaping/quoting with round-trip tests.

4. Setup defaults container to root user

   • Locations: cmd/msgvault/cmd/setup.go:281
   • Generated docker-compose.yml runs as root by default, increasing impact of container compromise.
   • Recommended fix: Default to non-root; make root explicit/opt-in (or clearly prompt/warn when required for Synology).

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

roborev: Combined Review

Verdict: PR has one High and multiple Medium issues that should be addressed before merge.

High

• Insecure config file permissions can expose secrets
  Refs: internal/config/config.go:243, internal/config/config.go:257
Config.Save() uses os.OpenFile(..., O_TRUNC, 0600), which does not fix existing permissive modes on pre-existing files (e.g., 0644). This can leave values like [server].api_key / [remote].api_key world-readable.
  Suggested fix: write to a new temp file with 0600 and atomic rename, or enforce mode with chmod/secure helper before or after write.

Medium

• Auth bypass for write endpoints when API key is unset
  Refs: internal/api/server.go:132, internal/api/server.go:188
  Auth middleware skips checks if api_key is empty; new mutating endpoints (e.g., token upload/account updates) can then be called unauthenticated by local actors.
  Suggested fix: require auth for mutating endpoints regardless, or fail startup when write endpoints are enabled without an API key.

• Unbounded limit enables resource-exhaustion (DoS) patterns
  Refs: internal/api/handlers.go:627, internal/api/handlers.go:717, internal/api/handlers.go:783, internal/api/handlers.go:~560, internal/api/handlers.go:~640
  Multiple handlers/parsers accept arbitrarily large positive limits, allowing expensive queries and large response generation.
  Suggested fix: enforce a hard max (commonly <= 500) across all relevant paths and test oversized inputs.

• /messages/filter pagination metadata appears incorrect
  Refs: internal/api/handlers.go:526, internal/api/handlers.go:958
  Response total is reported as page length (len(summaries)) rather than total matching rows, which breaks pagination semantics for clients.
  Suggested fix: return true total count, or rename/document field semantics (e.g., returned_count).

• Remote query reconstruction can change search semantics
  Refs: internal/remote/engine.go:650, internal/remote/engine.go:665, internal/remote/engine.go:678, internal/remote/engine.go:687
  Rebuilt query strings do not reliably re-quote multi-word terms (e.g., subject:"foo bar" can become subject:foo bar), altering parser behavior.
  Suggested fix: quote/escape terms during reconstruction and add round-trip tests for quoted/mixed queries.

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

roborev: Combined Review

Verdict: Not ready to merge; reviewers identified 1 High and 5 Medium-severity issues.

High

1. API can start without enforced auth key (fail-open risk)

• Files: cmd/msgvault/cmd/serve.go (function runServe), internal/api/server.go
• Issue: Server startup does not explicitly fail when cfg.Server.APIKey is empty, which may expose sensitive endpoints without authentication.
• Suggested fix: Fail secure on empty API key (or strictly bind to localhost with explicit warning), and ensure auth middleware rejects requests when server API key is unset.

Medium

1. Unbounded limit enables DoS via large result sets

• Files/lines: internal/api/handlers.go:717, internal/api/handlers.go:783, internal/api/handlers.go:785, internal/api/handlers.go:936
• Issue: parseMessageFilter / aggregate parsing accepts any positive limit without a hard cap, allowing expensive CPU/memory/JSON work.
• Suggested fix: Enforce server-side maximum (for example <=500) and reject oversized requests.

2. Config save may leave sensitive file readable by other users

• File/line: internal/config/config.go:257
• Issue: Config.Save() uses OpenFile(..., 0600) with truncate; existing file modes are not corrected, so previously permissive config.toml can remain readable.
• Suggested fix: Enforce chmod 0600 on save and prefer temp-file write + atomic rename.

3. TUI --local flag behavior diverges from global mode handling

• Files/lines: cmd/msgvault/cmd/tui.go:64, cmd/msgvault/cmd/tui.go:236, cmd/msgvault/cmd/root.go:150
• Issue: TUI uses a separate --local path (forceLocalTUI) instead of shared global mode logic, creating inconsistent CLI behavior.
• Suggested fix: Use shared useLocal / IsRemoteMode() flow and remove duplicate flag semantics.

4. Fast-search default limit logic is inconsistent

• Files/lines: internal/api/handlers.go:788, internal/api/handlers.go:1034, internal/api/handlers.go:1037
• Issue: Parser default (500) prevents /search/fast from using its intended default (100) when limit is omitted.
• Suggested fix: Let endpoint-specific handlers apply defaults, or distinguish “not provided” from “provided”.

5. /messages/filter returns page-size count as total

• File/line: internal/api/handlers.go:958
• Issue: Response uses total: len(summaries) (current page length) instead of full match count, breaking expected pagination semantics.
• Suggested fix: Return full-match total (or rename field if intentionally page-local).

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

If there are security concerns it keeps raising that don't make sense, feel free to add stuff to review_guidelines in .roborev.toml

@wesm @hughbien

This PR implements TUI remote support (Issue #130) - enabling msgvault tui to work against a remote NAS when [remote].url is configured.

Changes:

• 6 new API endpoints for TUI aggregate queries (/api/v1/aggregates, /api/v1/search/fast, etc.)
• remote.Engine implementing query.Engine interface over HTTP
• --local flag to force local mode; deletion/export disabled remotely for safety

Status: All tests pass locally. Just pushed a lint fix - waiting on CI.

Ready for review when CI is green.

roborev: Combined Review

Verdict: 4 Medium-severity issues should be addressed before merge (1 availability risk, 3 regressions).

Medium

1. Unbounded limit enables expensive queries (DoS risk)
   Refs: internal/api/handlers.go:717, internal/api/handlers.go:736, internal/api/handlers.go:783, internal/api/handlers.go:792, internal/api/handlers.go:875, internal/api/handlers.go:918, internal/api/handlers.go:946
parseAggregateOptions / parseMessageFilter accept very large positive limit values that flow into query execution for aggregates and filtered messages, allowing oversized scans/responses.

2. Fast-search default limit regression (doc/behavior mismatch)
   Refs: internal/api/handlers.go:788, internal/api/handlers.go:1035
parseMessageFilter defaults limit=500; handleFastSearch only overrides when limit==0 || limit>500, so fast search defaults to 500 instead of documented 100.

3. Remote search query reconstruction loses phrase semantics
   Refs: internal/remote/engine.go:650, internal/remote/engine.go:664, internal/remote/engine.go:667, internal/remote/engine.go:680
buildSearchQueryString rebuilds terms without robust quoting/escaping, so multi-word terms/phrases can be re-parsed with different meaning.

4. No SQLite fallback in serve when DuckDB/parquet path fails
   Refs: cmd/msgvault/cmd/serve.go:88, internal/query/duckdb.go:1018, internal/query/duckdb.go:1449
   Server wiring can leave remote query endpoints failing at runtime instead of degrading to SQLite when parquet/extension setup is unavailable.

Synthesized from 4 reviews (agents: gemini, codex | types: security, default)

roborev: Combined Review

Verdict: Not ready to merge; there are multiple Medium-severity issues that should be fixed first.

Critical

• None.

High

• None.

Medium

1. Unbounded limit allows authenticated resource exhaustion (DoS)

   • Locations: internal/api/handlers.go:135, internal/api/handlers.go:201, internal/api/handlers.go:132 (approx), internal/api/handlers.go:200 (approx), internal/api/handlers.go:~706, internal/api/handlers.go:~717, internal/api/handlers.go:~753, internal/api/handlers.go:~783, internal/api/handlers.go:~938
   • Details: parseMessageFilter / parseAggregateOptions accept very large limit values without a hard cap, and downstream handlers (handleFilteredMessages, handleAggregates, handleSubAggregates) can execute expensive large-result queries.
   • Suggested fix: Enforce a server-side max (e.g., 500/1000) in the parsing helpers and reject oversized values (or clamp consistently).

2. Deep-search runtime regression when DuckDB SQLite extension is unavailable

   • Locations: cmd/msgvault/cmd/serve.go:88, internal/query/duckdb.go:1454
   • Details: serve initializes DuckDB with sqliteDB=nil, which can cause deep search to fail at runtime (Search requires SQLite) instead of gracefully falling back when sqlite_scanner is not available.
   • Suggested fix: Pass the live SQLite handle (s.DB()) into NewDuckDBEngine so fallback paths remain functional.

3. Pagination semantics bug in filtered messages response

   • Location: internal/api/handlers.go:958
   • Details: /api/v1/messages/filter returns "total": len(summaries) (page size), not total matching rows, which breaks expected pagination behavior.
   • Suggested fix: Return actual total-match count (separate count query) or rename the field to reflect returned-page count.

4. Missing tests for new remote engine logic

   • Location: internal/remote/engine.go
   • Details: New remote engine request/response logic was flagged as untested, increasing regression risk in critical client/server integration paths.
   • Suggested fix: Add focused unit tests (e.g., internal/remote/engine_test.go) for request construction, error handling, and response parsing.

Synthesized from 4 reviews (agents: gemini, codex | types: security, default)

roborev: Combined Review (f5be139)

Verdict: The proposed changes add remote TUI support and HTTP endpoints, but
introduce a compilation error and a functional regression that drops context filters during deep searches.

High

• cmd/msgvault/cmd/serve.go (around line 90)

  Missing the fourth argument (query.DuckDBOptions) in the call to query.NewDuckDBEngine, which will cause a compilation error (as evidenced by the 4-argument call in tui.go).
  Suggested fix: Pass an empty options struct as the fourth argument:
  duckEngine, engineErr := query.NewDuckDBEngine(analyticsDir, dbPath, s.DB(), query.DuckDBOptions{})

Medium

• internal/api/handlers.go
  and internal/remote/engine.go
  Remote deep search drops TUI context filters. remote.Engine.Search only sends q/offset/limit, and handleDeepSearch only
  parses q, so account-scoped search, hide_deleted, and any other drill-down filters that are not representable in the raw query string are lost. In practice, a remote deep search can return messages outside the current account/context.
  Suggested fix: Let /api/v 1/search/deep accept the same filter params as the fast-search path, parse them with parseMessageFilter, merge them into the query before calling s.engine.Search, and add regression tests for source_id / hide_deleted / drill-down filters.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (5ac304b)

Summary Verdict: The PR successfully introduces remote-TUI HTTP API endpoints and a remote query engine client, but contains
medium-severity issues with dropped drill-down filters and incorrect default pagination sizes.

Medium

1. Remote deep search still drops several drill-down filters
Files: [internal/api/handlers.go](/home/roborev/repos/msgvault/internal/api/handlers
.go), internal/query/sqlite.go#L1454
handleDeepSearch parses the full MessageFilter, but then collapses it through query. MergeFilterIntoQuery(...) before calling engine.Search(...). That merger only carries SourceID, sender, recipient, label, attachments, domain, and hide-deleted. It does not preserve TimeRange, SenderName, RecipientName, ConversationID, or EmptyValueTargets. As
a result, remote deep search can escape the current time bucket / name drill-down / empty-bucket context and return broader results than the TUI is showing.
Suggested fix: Either extend the merge/search model to represent all supported filters, or add a deep-search engine/API path that accepts
MessageFilter directly instead of forcing everything through search.Query. Tests are missing for deep search inside time/name/thread contexts.

2. The shared filter parser gives the new search endpoints the wrong default page size
File: [internal/api/handlers.go](/home/
roborev/repos/msgvault/internal/api/handlers.go)
parseMessageFilter unconditionally sets Pagination.Limit = 500 when the query omits limit. handleFastSearch and handleDeepSearch then try to apply their own 100 -row default, but they only do that when limit == 0 or limit > 500. Since the parser has already converted "unset" to 500, both endpoints now default to 500, not 100 as documented. This also makes count-
only fast searches expensive: callers using limit=0 to mean "count only" still end up executing a 500-row page fetch.
Suggested fix: Leave Limit unset in parseMessageFilter and let each endpoint apply its own default/clamp, or pass the desired
default into the parser. Add tests for omitted limit and limit=0.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (06668d6)

The PR successfully introduces the remote TUI and HTTP API surface, but requires fixes for a compilation error, unconstrained API limits, missing date filters, and potential stale cache reads in daemon mode.

High

• cmd/
  msgvault/cmd/serve.go (~line 92)
  query.NewDuckDBEngine is called with 3 arguments (analyticsDir, dbPath, s.DB()), but
  its usage in cmd/msgvault/cmd/tui.go shows it requires a 4th argument of type query.DuckDBOptions. This missing argument will result in a compilation error.
  Suggested fix: Pass an empty options struct as the 4th argument: query.New DuckDBEngine(analyticsDir, dbPath, s.DB(), query.DuckDBOptions{}).

Medium

• cmd/msgvault/cmd/serve.go (compared with
  cmd/msgvault/cmd/tui.go:77)
  The new daemon path selects DuckDB as soon as HasCompleteParquetData(...) is true, but it does not run the same cacheNeedsBuild freshness check that the local tui uses before opening DuckDB. That means remote aggregates, message lists, and fast search can serve stale Parquet data after startup if the cache is outdated.
  Suggested fix: Reuse cacheNeedsBuild in serve startup and either rebuild incrementally or force SQLite until the cache is fresh.

• internal/api/handlers.go#L726, L7
  99, L945
  The new remote
  TUI endpoints accept caller-controlled limit values without enforcing the documented upper bounds. /messages/filter promotes any positive limit directly into the backend query, then increments it by one for pagination. On a remotely reachable daemon, an attacker can request very large pages and force high memory usage, large JSON responses, and
  expensive backend scans, which is a practical availability issue.
  Suggested fix: Clamp limit server-side for every new endpoint. Cap /messages/filter, /search/*, and aggregate endpoints to a small maximum such as 500, and reject oversized values with 400 Bad Request.

• internal/query/models.go:212, [internal/query/models.go:288](/home/roborev/repos/msgvault/internal/
  query/models.go:288), internal/api/handlers.go, [internal/remote/engine.go](/home/roborev/repos/msgvault/internal/
  remote/engine.go)
  The query contract supports absolute date bounds via MessageFilter.After/Before and AggregateOptions.After/Before. Local SQLite/DuckDB engines honor them, but the new HTTP transport does not encode or parse those fields for list/aggregate/stats requests. In remote mode
  , the same filter set can therefore return broader results than local mode.
  Suggested fix: Add after/before query params end-to-end and cover them with remote-engine and handler tests.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (2929cd3)

Summary Verdict: The PR introduces a compilation error in the serve command and a silent truncation bug in the remote thread view.

High

Missing parameter causes compilation error

• Location: cmd/msgvault/cmd/serve.go:8 8
• Description: Missing the 4th parameter (duckOpts) in the call to query.NewDuckDBEngine. In tui.go, this function is called with 4 arguments. Omitting the 4th argument here will cause a compilation error.
• Suggested Fix
  : Pass an empty options struct or the default options: query.NewDuckDBEngine(analyticsDir, dbPath, s.DB(), query.DuckDBOptions{}).

Medium

Remote thread view can silently truncate threads above 500 messages

• Location: internal/api/handlers .go, internal/tui/model.go
• Description: The new API enforces a hard maxPageSize = 500 and clamps limit in parseMessageFilter, but the TUI thread loader still asks for threadLimit + 1 with a default
  thread limit of 1000 to detect truncation. In remote mode, a 700-message thread comes back as 500 messages, and loadThreadMessages never sets truncated because it only checks len(messages) > threadLimit. Incomplete thread data is thus shown as if
  it were complete.
• Suggested Fix: Either raise the API cap for conversation_id thread fetches, or return/propagate has_more through remote.Engine.ListMessages so the thread view can detect truncation even when the server trims the result set. (Note: there is currently no test
  coverage for remote thread loading when a conversation exceeds 500 messages).

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (2d435b9)

Summary Verdict: The PR successfully implements remote TUI
and background daemon capabilities, but requires fixes for a compilation error, a deep-search context bug, and missing bounds checks on API endpoints.

High

• cmd/msgvault/cmd/serve.go:94
query.NewDuckDBEngine is called with 3
  arguments, but its signature requires 4 arguments. This will cause a compilation error.
  Fix: Add a zero-value query.DuckDBOptions{} as the fourth argument: query.NewDuckDBEngine(analyticsDir, dbPath, s.DB(), query.DuckDBOptions{}).

Medium

• internal/api/handlers.go, internal/query/sqlite.go#L1454, internal/tui/model.go#L485
  Deep search still drops several drill-down filters. handleDeepSearch parses a full MessageFilter, but it then reduces everything to search.Query via MergeFilterIntoQuery. That type cannot carry sender_name, recipient_name, time_period, conversation_id, or empty_targets, so deep search from those contexts can escape the current
  drill-down and return unrelated messages.
  Fix: Add a filter-aware deep-search path in the engine/API instead of re-encoding context into search.Query.

• internal/api/handlers.go (in parseAggregateOptions and handleFastSearch)

  The new remote TUI endpoints accept caller-controlled limit values without the hard cap used by /messages/filter and /search/deep. An authenticated client can request extremely large result sets and force expensive scans plus large JSON responses, leading to potential resource exhaustion/availability issues.
  Fix:
  Clamp these endpoints to a fixed upper bound before invoking the engine, ideally reusing maxPageSize or a smaller endpoint-specific cap. Reject oversized values with 400 Bad Request.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (466fccf)

Summary Verdict: The PR successfully implements remote TUI support and query-backed API endpoints, but contains a medium-severity issue where deep search silently drops certain filters.

Medium

internal/api/handlers.go, internal/query/sqlite.go
/api/v1/search/deep parses sender_name, recipient_name, time_period, conversation_id, and empty_targets, but handleDeepSearch then funnels everything through MergeFilterIntoQuery, which explicitly drops those fields. That means those filters silently do nothing for deep search, and T
UI deep-search from thread/time/name/empty-bucket drilldowns can escape the current scope. Suggested fix: carry MessageFilter through deep search end-to-end instead of collapsing to search.Query, or reject unsupported params until they are implemented.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (7429f4d)

Verdict: The PR successfully implements remote TUI support and query
API endpoints with sound security boundaries, but contains a medium-severity bug where fast search escapes drill-down scopes.

Medium

Fast search escapes drill-down scope for specific filters
handleFastSearch accepts a full MessageFilter in internal/api/handlers.go#L10
93, but the backend engines do not honor all of its parameters.

• The DuckDB fast-search path only applies a subset in internal/query/duckdb.go#L2
  002.
• The SQLite path drops SenderName, RecipientName, ConversationID, and EmptyValueTargets in internal/query/sqlite.go#
  L1336.

Since fast search is the default TUI mode, searching from sender-name, recipient-name, empty-bucket, or thread drill-downs can incorrectly return results outside the
visible scope.

Suggested Fix: Either explicitly reject unsupported filters in handleFastSearch (similar to how handleDeepSearch operates) or implement the missing filters in both the DuckDB and SQLite engines. Add tests to cover these specific context drill-downs.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

roborev: Combined Review (23fa7b5)

Summary: The PR introduces a remote query engine and HTTP endpoints but contains a few medium-severity logical and cache-invalidation issues that need addressing.

Medium

1. Stale DuckDB data after deletion-only scheduled syncs
Files: /home/roborev/repos/ msgvault/cmd/msgvault/cmd/serve.go (lines 320-331)
Description: runScheduledSync only rebuilds the Parquet cache when summary.MessagesAdded > 0. Incremental syncs can mark messages deleted without adding any new mail
, and this stack explicitly treats post-build deletions as cache-staleness. In serve mode, the engine is chosen once at startup; after a deletion-only sync, a running DuckDB-backed server will keep returning stale rows until restart or until a later sync adds mail.
Suggested Fix: Trigger cache rebuild
on any sync that mutates indexed data, not just additions. At minimum, track deletion/update counts in the sync summary and rebuild on those too. Add an integration-level test around deletion-only scheduled syncs/cache refresh.

2. SQLite fallback silently ignores time_period in fast search

Files: /home/roborev/repos/msgvault/internal/api/handlers.go (lines 1105-1117), /home/roborev/repos/msgvault/internal/query/sqlite.go (lines 1336-
1339)
Description: parseMessageFilter populates filter.TimeRange, but handleFastSearch only rejects sender_name, recipient_name, conversation_id, and empty_targets. When the server falls back to SQLiteEngine (which
serve does whenever the cache is missing/stale), fast search goes through MergeFilterIntoQuery, and that function explicitly does not merge TimeRange. Result: fast search from a remote time-bucket drilldown can escape the current bucket with no error.
Suggested Fix: Either reject time_ period in handleFastSearch the same way deep search does, or teach the SQLite fast-search path to honor TimeRange directly. Add a handler test for fast search with time_period on SQLite fallback.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@EconoBen sorry I've been delayed! This branch is looking pretty good now. I will update my remote msgvault deployment and try this out and plan to get a 0.10.0 release out with this plus the IMAP support in the next 24 hours

roborev: Combined Review (ddcf049)

Summary Verdict: The new remote TUI and API changes are well-structured and secure, but introduce two medium-severity cache coherency issues during
incremental syncs that need addressing.

Medium

• Deleted messages left in cache during mixed syncs: Mixed add/delete syncs can still leave deleted messages in the Parquet cache. In cmd/msgvault/cmd/tui.go, cacheNeedsBuild returns immediately when max ID > state.LastMessageID, so it never checks for post-watermark deletions in the same sync. Then cmd/msgvault/cmd/serve.go decides fullRebuild by checking whether the human-readable reason string contains "deletions". If a sync both adds mail and deletes
  older mail, the reason becomes "N new messages", the rebuild stays incremental, and old deleted rows remain in Parquet. Return structured staleness flags instead of parsing the reason string, and force full rebuild whenever any deletion is detected.

• Label updates missing from analytics cache: Label-only incremental syncs are still
  not reflected in the analytics cache. cmd/msgvault/cmd/serve.go now claims the post-sync rebuild path covers label updates, but cmd/msgvault/cmd/tui.go only checks new message IDs and deletions. Meanwhile cmd/msgvault/cmd/build_ cache.go exports message_labels incrementally only for message_id > lastMessageID, so label add/remove events on existing messages never trigger or get picked up by an incremental rebuild. That leaves remote aggregates and label filters stale after ordinary History API label changes. Track a watermark for label mutations or
  force a full rebuild when existing-message labels change.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

I have checked this out with my own remote deployment and just waiting for a clean bill of health to merge

went ahead and merged this. I'm going to work on the 0.10.0 release notes